Skip to content

Initial release #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 55 commits into from
Jul 9, 2020
Merged

Initial release #1

merged 55 commits into from
Jul 9, 2020

Conversation

@rchougule
Copy link
Contributor

@rchougule rchougule commented Jun 30, 2020

First release of the tool

rchougule added 30 commits May 29, 2020 14:24
@rchougule
Network Utility Tool in a separate folder. Yet to add specs.
c96daca
@rchougule
unit test setup using mocha, chai and sinon. utils half covered. Util…
4f3645b 
…s fixes.
@rchougule
more tests for utils
7ddf0c1
@rchougule
spec for mac stats partial.
f8fa072
@rchougule
fixed minor and patch versions for dev deps
536fa9d
@rchougule
mac network stats unit test
cb97f78
@rchougule
more specs and fixes
99954d3
@rchougule
nock for mocking http requests. connectivity check without proxy tests.
a93ffde
@rchougule
connectivity checks specs with proxy cases and error case
4cb990f
@rchougule
specs for server done. win mem stats specs remain. dummy initializers…
a4ba616 
… for loggers, handlers etc for testing. specs for stats covered
@rchougule
windows mem stats specs add
3ce1015
@rchougule
detecting win32 via regex for win systems
ab17047
@rchougule
comments across files and individual functions. CLI Command usage exp…
9edd094 
…lanation with --help argument
@rchougule
EOLs
704542e
@rchougule
npm start script
735e977
@rchougule
pkg downgrade to support node 4. added the executables for all platfo…
e69adbd 
…rms.
@rchougule
timeout increased for connectivity checks. readme added. cli help upd…
5d220ae 
…ated
@rchougule
readme updated for steps to build the executables
dc40734
@rchougule
logging improved while starting the tool
50f77c8
@rchougule
custom logs directory. improved args helper. specs added for the chan…
859db8e 
…ges. error handling for stats
@rchougule
better error messaging in case path for logs directory is invalid
746cc06
@rchougule
condition refactor for logs directory error
13ea1ae
@rchougule
helper for specs moved to testHelper. Better messaging while initiati…
1a7f51e 
…ng the proxy.
@rchougule
linter setup. readme update
02587ed
@rchougule
EOF. Linter rule for EOF
7752471
@rchougule
build scripts in package.json. README update
f43f54c
@rchougule
nyc exclude folders
8425060
@rchougule
retries for request fired by the tool. Yet to decide the status codes…
7095c03 
… for retry to initiate
@rchougule
Migration
f8bb5bd
@rchougule
Merge branch from source into initial-release
619f4d0
yashLadha
yashLadha reviewed Jul 1, 2020
View reviewed changes
Copy link

@yashLadha yashLadha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some concerns.

@yashLadha
Copy link

yashLadha commented Jul 1, 2020 •
edited
Loading

Also, as this is an HTTP proxy what about https requests or any other protocol requests? How are we planning to handle that.

AmeyaJoshi1
AmeyaJoshi1 reviewed Jul 1, 2020
View reviewed changes
@rchougule
Copy link
Contributor Author

rchougule commented Jul 1, 2020

Also, as this is an HTTP proxy what about https requests or any other protocol requests? How are we planning to handle that.

That was a conscious call to make this a HTTP proxy only. Reasons :

  1. HTTPS proxy will require self signed certs to be added in the users Keychain, just like mitm proxy & browsermob. We didn't want to do that with this release.
  2. Another approach was to tap into TCP level sockets and not Application level (HTTP, HTTPS). Thus, connecting via the TCP level sockets directly. But in that case, the proxy would have been of no use since it would not have been able to read the intercepted request. That's because, a direct socket connection in case of HTTPS request would have encrypted the data already.

rchougule and others added 2 commits July 1, 2020 18:54
@rchougule @AmeyaJoshi1
Installation instruction updated
66c5b41 
Co-authored-by: Ameya Joshi <josameya@gmail.com>
@rchougule
configurable timeout for requests fired by the tool. specs for the ti…
9b9bd0c 
…meout and retry delay. README update for the same
AmeyaJoshi1
AmeyaJoshi1 reviewed Jul 1, 2020
View reviewed changes
AmeyaJoshi1
AmeyaJoshi1 reviewed Jul 1, 2020
View reviewed changes
AmeyaJoshi1
AmeyaJoshi1 reviewed Jul 1, 2020
View reviewed changes
Archish27
Archish27 reviewed Jul 1, 2020
View reviewed changes
Copy link

@Archish27 Archish27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@rchougule
proxy-port check in expected range. tests for the same. collect netwo…
55a73cd 
…rk & connectivity logs when any request fails. readme update
DravitLochan
DravitLochan reviewed Jul 1, 2020
View reviewed changes
Copy link

@DravitLochan DravitLochan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

functionality wise looks good. some suggestions.

DravitLochan
DravitLochan reviewed Jul 2, 2020
View reviewed changes
@DravitLochan
Copy link

DravitLochan commented Jul 2, 2020 •
edited
Loading

node.js can be renamed. app.js/ index.js / requestDebugger.js

@rchougule
Copy link
Contributor Author

rchougule commented Jul 2, 2020

node.js can be renamed. app.js/ index.js / requestDebugger.js

renamed to requestsDebugger.js

DravitLochan
DravitLochan reviewed Jul 2, 2020
View reviewed changes
DravitLochan
DravitLochan previously approved these changes Jul 2, 2020
View reviewed changes
Copy link

@DravitLochan DravitLochan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@francisf francisf merged commit 9e10f3c into browserstack:master Jul 9, 2020
@rchougule rchougule deleted the initial-release branch July 16, 2020 13:26
francisf pushed a commit that referenced this pull request Oct 27, 2020
@neerajshukla1911
Merge pull request #1 from rchougule/reverse-proxy-with-https
37e90bb 
Github Actions setup for unit tests. Logo and badge addition in README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@francisf francisf francisf approved these changes

+4 more reviewers

@AmeyaJoshi1 AmeyaJoshi1 AmeyaJoshi1 left review comments

@Archish27 Archish27 Archish27 left review comments

@yashLadha yashLadha yashLadha left review comments

@DravitLochan DravitLochan DravitLochan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@rchougule @yashLadha @DravitLochan @AmeyaJoshi1 @Archish27 @francisf